Modified ad blocking test flow #5731
Conversation
niklasberglund
force-pushed
the
race-condition-in-ad-blocking-test-ios-466
branch
2 times, most recently
from
5c8fb71 to
a5f8588
Compare
niklasberglund
changed the title
niklasberglund
force-pushed
the
race-condition-in-ad-blocking-test-ios-466
branch
from
a5f8588 to
9bd0d77
Compare
There was a problem hiding this comment.
Reviewed all commit messages.
Reviewable status: 0 of 3 files reviewed, 2 unresolved discussions (waiting on @niklasberglund)
ios/MullvadVPNUITests/RelayTests.swift line 14 at r3 (raw file):
class RelayTests: LoggedInWithTimeUITestCase { func testAdBlockingViaDNS() throws { TunnelControlPage(app)
Don't forget to verifyCanReachAdServingDomain before we run the test, otherwise we could get false positives if a previous run left ad blocking turned on for whatever reason
ios/MullvadVPNUITests/RelayTests.swift line 15 at r3 (raw file):
func testAdBlockingViaDNS() throws { TunnelControlPage(app) .tapSettingsButton()
🔴 Value of type 'TunnelControlPage' has no member 'tapSettingsButton'
niklasberglund
force-pushed
the
race-condition-in-ad-blocking-test-ios-466
branch
from
9bd0d77 to
18abea9
Compare
There was a problem hiding this comment.
Reviewable status: 0 of 3 files reviewed, 2 unresolved discussions (waiting on @buggmagnet and @pinkisemils)
ios/MullvadVPNUITests/RelayTests.swift line 14 at r3 (raw file):
Previously, buggmagnet wrote…
Don't forget to
verifyCanReachAdServingDomainbefore we run the test, otherwise we could get false positives if a previous run left ad blocking turned on for whatever reason
Brought this up with @pinkisemils before implementing this, we talked about not including verifyCanReachAdServingDomain since with this test flow the app is never in a state of being both connected and having ad blocking turned on. So even if ad blocking was left on from a previous run it wouldn't catch this, or would it even though we're not connected to a relay?
ios/MullvadVPNUITests/RelayTests.swift line 15 at r3 (raw file):
Previously, buggmagnet wrote…
🔴 Value of type 'TunnelControlPage' has no member 'tapSettingsButton'
Oh yes, I must have forgotten to re-run the test after solving merge conflicts. It has moved to HeaderBar. Pushed a fix.
There was a problem hiding this comment.
Reviewed 1 of 3 files at r2, 1 of 2 files at r3, 1 of 1 files at r4, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @pinkisemils)
ios/MullvadVPNUITests/RelayTests.swift line 14 at r3 (raw file):
Previously, niklasberglund (Niklas Berglund) wrote…
Brought this up with @pinkisemils before implementing this, we talked about not including
verifyCanReachAdServingDomainsince with this test flow the app is never in a state of being both connected and having ad blocking turned on. So even if ad blocking was left on from a previous run it wouldn't catch this, or would it even though we're not connected to a relay?
Right, we uninstall the app before starting the test here, good point then !
buggmagnet
force-pushed
the
race-condition-in-ad-blocking-test-ios-466
branch
from
18abea9 to
5098c25
Compare
In the ad blocking test there's a race condition when enabling ad blocking and then verifying that an ad blocking domain cannot be reached. The problem is that changing the setting triggers a reconnect and there is no visual update in the app to wait for in order to make sure we're connected again before verifying that the ad blocking domain no longer can be reached.
We decided to modify the test flow to:
See issue in Linear https://linear.app/mullvad/issue/IOS-466/race-condition-in-ad-blocking-test
The changes in this PR can be tested by running the
testAdBlockingViaDNStest.This change is